Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add variant: NUCLEO-H503RB. #2092

Closed
wants to merge 2 commits into from
Closed

Add variant: NUCLEO-H503RB. #2092

wants to merge 2 commits into from

Conversation

dojyorin
Copy link
Contributor

@dojyorin dojyorin commented Aug 5, 2023 •
edited
Loading

Add variant: NUCLEO-H503RB

Summary

This PR fixes/implements the following bugs/features

@fpistm feat adds STM32H5 core implementation and NUCLEO-H563ZI support (That was really great help! Thank you!), but variant support is still lacking.

This time, I'll add new H5 NUCLEO board NUCLEO-H503RB with this pull request.
Related to #722 (comment) and #2091

Progress

There's still some work to do.

Basically implementation of NUCLEO-H563ZI is used reference.

I'm new to STM32, so excuse me if I'm doing something wrong.
I would like to discuss with everyone and produce good results.

@fpistm fpistm self-requested a review August 5, 2023 18:12
@fpistm fpistm added the new variant Add support of new bard label Aug 5, 2023
@fpistm fpistm added this to the 2.7.0 milestone Aug 5, 2023
@fpistm
Copy link
Member

fpistm commented Aug 5, 2023

Thanks for squashing. Anyway simply force update on the same branch will avoid creating new PR. :wink : Let ci run to check this PR first.

fpistm
fpistm requested changes Aug 7, 2023
View reviewed changes
Copy link
Member

@fpistm fpistm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First thanks for this PR.
In addition to my comments, could you add the generic and the Nucleo in the README.md and also declare them to the boards.txt.
Finally, split in 2 commit, 1 for the generic addition and one for the dedicated Nucleo board.

You can refers to this PR as example:
#1859

variants/STM32H5xx/H503RBT/variant_NUCLEO_H503RB.h Show resolved Hide resolved
variants/STM32H5xx/H503RBT/variant_NUCLEO_H503RB.h Outdated Show resolved Hide resolved
variants/STM32H5xx/H503RBT/variant_NUCLEO_H503RB.h Outdated Show resolved Hide resolved
variants/STM32H5xx/H503RBT/variant_NUCLEO_H503RB.h Outdated Show resolved Hide resolved
variants/STM32H5xx/H503RBT/variant_NUCLEO_H503RB.h Outdated Show resolved Hide resolved
variants/STM32H5xx/H503RBT/generic_clock.c Outdated Show resolved Hide resolved
variants/STM32H5xx/H503RBT/variant_NUCLEO_H503RB.cpp Outdated Show resolved Hide resolved
@dojyorin
Copy link
Contributor Author

Thank you for detailed explanation, it really helped me.
It was a great reference when I wanted to add other boards in the future.

For the time being, I've fixed the part you told me, so I'll commit it.

@dojyorin dojyorin requested a review from fpistm August 15, 2023 07:50
@dojyorin
Copy link
Contributor Author

The Arduino pinout only goes up to D15/A5, but the MCU has more pins.
I wasn't sure what rules to define the "surplus D16/A6 and beyond" pins.

@fpistm
Copy link
Member

fpistm commented Aug 28, 2023 •
edited
Loading

Hi @dojyorin
I've reworked and fixed your PR.
https://github.com/fpistm/Arduino_Core_STM32/tree/feat-nucleo-h503rb
Could you try it and force update to your branch. Thanks

@fpistm fpistm mentioned this pull request Aug 30, 2023
Merged
@fpistm fpistm added the abandoned No more work on this label Aug 30, 2023
@fpistm fpistm closed this Aug 30, 2023
@dojyorin dojyorin deleted the feat-nucleo-h503rb branch September 11, 2023 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fpistm fpistm Awaiting requested review from fpistm

Assignees
No one assigned
Labels
abandoned No more work on this new variant Add support of new bard
Projects
STM32 core based on ST HAL
Status: Done
Milestone
2.7.0
Development

Successfully merging this pull request may close these issues.
2 participants
@dojyorin @fpistm